Repository navigation
fix(@angular/build): rebuild stylesheets when files are added to a PostCSS directory dependency - #34451
fix(@angular/build): rebuild stylesheets when files are added to a PostCSS directory dependency#34451MeAkib wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for directory dependencies (such as PostCSS dir-dependency) by watching the dependency directory itself and updating the cache invalidation logic to check parent directories recursively. The review feedback suggests normalizing the input path in the invalidate method to ensure OS-consistent path matching when looking up file dependencies.
22295e9 to
8462bbf
Compare
alan-agius4
added
the
action: review
alan-agius4
added
the
target: patch
…stCSS directory dependency Previously, a PostCSS `dir-dependency` message (used by Tailwind CSS v4 for its content sources) was expanded with a glob and only the files that existed at build time were watched. Files created later, or moved by renaming their folder, did not invalidate the cached stylesheet, so classes used only in them were not generated until the stylesheet itself changed. The directory is now also added to the watch files, and the load result cache invalidates an entry when a changed path is inside one of its watched directories. Fixes angular#34446
8462bbf to
3a72951
Compare
alan-agius4
removed
the
action: review
alan-agius4
left a comment
alan-agius4
left a comment
There was a problem hiding this comment.
Please see the inline comments.
| ) { | ||
| loadResult.watchFiles ??= []; | ||
| // Watch the directory itself so files added or moved into it later also cause a rebuild | ||
| loadResult.watchFiles.push(resultMessage['dir']); |
There was a problem hiding this comment.
Adding resultMessage['dir'] directly to watchFiles causes a few issues:
watchFilesis intended for file paths rather than directories (for examplePersistentLoadResultCachecallsreadFileon each entry inwatchFiles, which fails withEISDIR).- It ignores
resultMessage['glob']during invalidation and watching. Whendirissrcor the workspace root (such as in Tailwind v4), modifying any file underdirwill trigger the watcher and invalidate the stylesheet cache, even for files that do not matchglob. - Because
glob()is still executed below, all existing files and the parent directory are both added towatchFiles.
| let invalid = false; | ||
|
|
||
| // Check parent directories to support directory dependencies (PostCSS `dir-dependency`) | ||
| let current = normalize(path); | ||
| while (true) { | ||
| const affectedPaths = this.#fileDependencies.get(current); | ||
| if (affectedPaths) { | ||
| for (const affected of affectedPaths) { | ||
| this.#loadResults.delete(affected); | ||
| } | ||
| invalid = true; | ||
| } | ||
|
|
||
| for (const affected of affectedPaths) { | ||
| this.#loadResults.delete(affected); | ||
| const parent = dirname(current); | ||
| if (parent === current) { | ||
| break; | ||
| } | ||
| current = parent; | ||
| } | ||
|
|
||
| return true; | ||
| return invalid; | ||
| } |
There was a problem hiding this comment.
Walking up dirname() to the filesystem root on every invalidate() call turns an O(1) lookup into an O(depth) walk for every changed file across all caches (including SourceFileCache and ComponentStylesheetBundler).
Also, after recent changes on main, BundlerContext.invalidate() checks !this.watchFiles.isDisjointFrom(candidateFiles) rather than the return value of this.#loadCache.invalidate(file). When a new file is created inside a watched directory, candidateFiles only has the new file path while this.watchFiles only has the directory path, so this.#esbuildResult is not cleared and the bundle is not rebuilt.
Lastly, normalize() in put() preserves trailing slashes on directory paths while dirname() strips them, so a dir with a trailing slash will not match.
|
|
||
| // Emits a class for each source file, similar to Tailwind CSS v4 content scanning | ||
| await harness.writeFile( | ||
| 'test-plugin.js', |
There was a problem hiding this comment.
Please use a unique plugin filename here instead of test-plugin.js. An earlier test in this suite also uses ./test-plugin.js in .postcssrc.json, and StylesheetPluginFactory caches PostCSS processors by config and require.cache, which can cause the wrong plugin to be reused.
|
Closing as this require a completely different approachl, likely integrate actual directory watching via the esbuild result objects. |
Previously, a PostCSS
dir-dependencymessage (used by Tailwind CSS v4 for its content sources) was expanded with a glob and only the files that existed at build time were watched. Files created later, or moved by renaming their folder, did not invalidate the cached stylesheet, so classes used only in them were not generated until the stylesheet itself changed.The directory is now also added to the watch files, and the load result cache invalidates an entry when a changed path is inside one of its watched directories.
Fixes #34446